Repository navigation
Conversation
Signed-off-by: David Leong <leongdl@amazon.com>
github-actions
Bot
added
the
waiting-on-maintainers
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| retired, at the same time as the hint-following classes below are enabled. | ||
| """Worker with session_runtime=service-selected defaults to python. | ||
|
|
||
| The service stamps runtimeHint=pythonexpr by default, but the worker only |
There was a problem hiding this comment.
The new premise here contradicts the skip reason on TestServiceSelectedWithPythonexprHint below. This docstring says the service already stamps runtimeHint=pythonexpr by default, but line 318-323 skips that class because it "Requires the test account to be in the intermediate allowlist state ... so the service stamps runtimeHint=pythonexpr" — i.e. it is gated on a precondition this docstring asserts is now satisfied. The module docstring (lines 25-26) carries the same stale claim.
If the pythonexpr hint is now stamped by default, TestServiceSelectedWithPythonexprHint should be un-skipped (it is the only test that actually pins hint='pythonexpr', and it would cover the branch this class stopped covering). If it is not reliably stamped, this docstring overstates it. Either way one of the two needs updating so a future reader can tell which state the account is in.
|
|
||
| The service stamps runtimeHint=pythonexpr by default, but the worker only | ||
| sees it when its botocore model has AssignedSession.metadata, so the hint | ||
| is None on some platforms. Both route to python, so the assertion does not |
There was a problem hiding this comment.
"the hint is None on some platforms" attributed to the botocore model looks like the wrong cause. The hint is read at scheduler.py:1170 from session_spec["metadata"]["runtimeHint"], and the botocore service model the agent uses is shipped with the package — it is identical on Linux and Windows, so it cannot make the hint present on one OS and absent on the other. If the hint really is intermittently absent, the cause is more likely a stale/vendored model override or the endpoint/region the test account resolves to, not the platform.
Since this whole diff exists to make the docstring match reality, it is worth naming the actual reason (or saying "absent in some environments — cause not yet pinned down"); a plausible-but-wrong explanation here will send the next person debugging a hint mismatch down the wrong path.
Signed-off-by: David Leong <leongdl@amazon.com>
leongdl
changed the title
Review guideTLDR: The service picks the runtime hint ( Where to look (one file, ~5 min):
What it catches:
The no-hint default is still covered by the unit tests in |
3 participants
What was the problem/requirement? (What/Why)
TestServiceSelectedDefaultsToPythonfails on the Windows canaries:Expected agent log to contain 'Selected session runtime: python (hint=None)'. The job succeeds; only the log assertion fails.The service now stamps a
runtimeHinton every session:pythonexprby default,rustfor allowlisted accounts. botocore 1.43.105 addedAssignedSession.metadatato the Deadline model. Older botocore dropped the field, so the agent always sawhint=None. ExpectingNoneonly matched how the service behaved during development. Choosing the hint is the service's job, and the agent's job is to route on it.What was the solution? (How)
TestServiceSelectedDefaultsToPythonand the two skipped hint classes (TestServiceSelectedWithRustHint,TestServiceSelectedWithPythonexprHint) with oneTestServiceSelectedFollowsServiceHint. It requirespython (hint='pythonexpr')orrust (hint='rust'), so the test fails if the hint is dropped anywhere between the service and the scheduler. The no-hint default stays covered bytest/unit/sessions/runtime/test_select.py._assert_log_containsnow accepts a sequence of patterns and passes if any one matches:grep -F -e … -e …on Linux, aSelect-String -Pattern 'a','b' -SimpleMatcharray on Windows. Single-string callers are unchanged.xfail(strict=False)on Linux. The Linux e2e worker runs Python 3.9, which caps botocore at 1.42.x, and 1.42.x has nometadata, so the hint never arrives there. Remove the mark once the Linux worker's botocore keeps the field; until then it reports XPASS.What is the impact of this change?
Test-only. Windows now asserts that the hint reaches the worker. Linux xfails until its botocore catches up. The test also stays correct through a later move to
rustwithout edits.How was this change tested?
ruff check,ruff format --check(pinnedruff ~= 0.15.22) andmypy --config-file test/e2e/mypy.inipass.python (hint='pythonexpr')rust (hint='rust')python (hint=None)python (hint='rust')(ignored hint)pytest --collect-onlywithOPERATING_SYSTEM=linuxandwindowscollects the new test. On Linux the xfail condition evaluates to true. The e2e suite itself runs in CI.Was this change documented?
Module and class docstrings updated.
Is this a breaking change?
No.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.